-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Mofed termination grace period param #443
Conversation
4dc1f30
to
ef06fb6
Compare
db07f6e
to
bb6ea4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM once CI passed
// +optional | ||
// +kubebuilder:default:=300 | ||
// +kubebuilder:validation:Minimum:=0 | ||
TerminationGracePeriodSeconds int `json:"terminationGracePeriodSeconds,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets make it the same as in k8s (i.e int64) WDYT ?
also do you see a benefit of having it a pointer vs non-pointer ?
for me, if we have default i think it fine to go with non-pointer type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets make it the same as in k8s (i.e int64) WDYT ?
Good catch, I agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to int64
--- | ||
apiVersion: apiextensions.k8s.io/v1 | ||
kind: CustomResourceDefinition | ||
metadata: | ||
annotations: | ||
controller-gen.kubebuilder.io/version: v0.4.1 | ||
controller-gen.kubebuilder.io/version: v0.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we really need to have some makefile targets to get a pre-defined version of controller-gen/operator-sdk so all generate with same version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a bug filed: #444
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version is defined here: https://github.com/Mellanox/network-operator/blob/master/Makefile#L248
The problem is that if you have an older version, it will not be updated unless you delete the older version from ./bin
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack so it just making sure CRD are updated using up2date version. in that case i dont think we need the bug.
we could technically add a CI step to regenerated crds and make sure no diffs or simillar
Adding to NicClusterPolicy CRD the possibility to specify the termination grace period for the Mofed container. The default is 300 seconds. It is also configurable in Helm values file. Signed-off-by: Fred Rolland <frolland@nvidia.com>
bb6ea4c
to
bc9535c
Compare
/retest_helm_ci |
/retest-nic_operator_helm |
Adding to NicClusterPolicy CRD the possibility to specify the termination grace period for the Mofed container. The default is 300 seconds.
It is also configurable in Helm values file.
Signed-off-by: Fred Rolland frolland@nvidia.com